Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Implement non-linear scaling for NDVI hybrid green correction #2554

Merged

Conversation

strandgren
Copy link
Collaborator

This PR extends on the NDVI hybrid green correction implemented in #2280 for improved True Color imagery with sensors having a green channel that misses the chlorophyll peak around 0.55 microns.

The previous implementation used a linear scaling from pixel-level NDVI to a fractional value determining how large fraction of the NIR signal that should be blended with the green signal to compute the hybrid green.

With this PR, the possibility to perform a non-linear scaling is implemented. The user defines the parameter strength, which controls the strength of the non-linearity.

The figure below show the behavior in Satpy main (linear scaling with no other option) as well as the behavior with this PR, where the scaling is non-linear with the shape and strength depending on the parameter strength.

image

It can be seen that by setting strength = 1.0 the current non-linear scaling is used. This is also the default in the compositor. A strength higher than 1.0 will result in a slower transition to higher/lower fractions at the NDVI extremes. Similarly, a strength smaller than 1.0 will result in a faster transition to higher/lower fractions at the NDVI extremes.

The new implementation used in this PR has been used for the release of the first FCI RGB imagery, and based on initial analysis a strength of 3.0 has been chosen as default for the ndvi_hybrid_green recipe used for the true_color. The same has been done for AHI (true_color_ndvi_green composite). As mentioned in the docstring, this value may change in the future when more data are available.

Some example results can be seen below:

Sensor Main This PR
FCI FCI_true_color_main FCI_true_color_strength-3 0
AHI AHI_true_color_ndvi_green_main AHI_true_color_ndvi_green_strength-3 0

From a first glance the images are rather similar, but it can be seen that with this PR more deep green colors (e.g. over Brazil) can be achieved, without having a reddening effect over the Sahara desert (which would be the case with the linear scaling). Similar patterns are seen for dense vegetation in south-east Asia and the barren surface in Australia.

  • Tests added
  • Fully documented

@codecov
Copy link

codecov bot commented Aug 24, 2023

Codecov Report

Merging #2554 (3767f61) into main (214e3f4) will increase coverage by 0.00%.
The diff coverage is 100.00%.

@@           Coverage Diff           @@
##             main    #2554   +/-   ##
=======================================
  Coverage   94.91%   94.91%           
=======================================
  Files         351      351           
  Lines       51217    51235   +18     
=======================================
+ Hits        48613    48631   +18     
  Misses       2604     2604           
Flag Coverage Δ
behaviourtests 4.29% <0.00%> (-0.01%) ⬇️
unittests 95.51% <100.00%> (+<0.01%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
satpy/composites/spectral.py 100.00% <100.00%> (ø)
satpy/tests/compositor_tests/test_spectral.py 100.00% <100.00%> (ø)

Copy link
Member

@simonrp84 simonrp84 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks good to me, no changes to suggest to the existing code.

That said, I currently have a PR open (#2555) that updates the AMI composites to use the existing NDVI hybrid green method. If acceptable, could we merge that one and then also update the AMI composites to make use of this new non-linear hybrid green?
Would be great if we can, but I can also make a separate PR for that later if preferred.

@strandgren
Copy link
Collaborator Author

@simonrp84 Thanks for reviewing! For me it's fine to first merge your PR and then I'd pull it and update the recipes based on the changes in my PR, if that's what your asking?

@simonrp84
Copy link
Member

Sorry @strandgren I wasn't clear! Yes, that's what I am asking :)

@mraspaud
Copy link
Member

mraspaud commented Sep 7, 2023

#2555 is now merged

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, just a couple of styling comments

satpy/composites/spectral.py Show resolved Hide resolved
satpy/tests/compositor_tests/test_spectral.py Outdated Show resolved Hide resolved
@strandgren
Copy link
Collaborator Author

This looks good to me, no changes to suggest to the existing code.

That said, I currently have a PR open (#2555) that updates the AMI composites to use the existing NDVI hybrid green method. If acceptable, could we merge that one and then also update the AMI composites to make use of this new non-linear hybrid green? Would be great if we can, but I can also make a separate PR for that later if preferred.

I've now merged the latest main (including the changes in #2555) and added the non-linear strength term to the AMI ndvi_hybrid_green recipes.

@coveralls
Copy link

Pull Request Test Coverage Report for Build 6224392120

  • 33 of 33 (100.0%) changed or added relevant lines in 2 files are covered.
  • No unchanged relevant lines lost coverage.
  • Overall coverage increased (+0.002%) to 95.468%

Totals Coverage Status
Change from base Build 6190601890: 0.002%
Covered Lines: 48744
Relevant Lines: 51058

💛 - Coveralls

Copy link
Member

@mraspaud mraspaud left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM!

@mraspaud mraspaud added enhancement code enhancements, features, improvements component:compositors labels Oct 10, 2023
@mraspaud mraspaud merged commit cefe7f8 into pytroll:main Oct 10, 2023
15 of 16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
component:compositors enhancement code enhancements, features, improvements
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants